Skip to content

Fixes footer with new sidebar changes#1619

Merged
Nemo157 merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:fixes-sidebar
Feb 13, 2022
Merged

Fixes footer with new sidebar changes#1619
Nemo157 merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:fixes-sidebar

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

Fixes #1613.

Since rust-lang/rust#92692, the sidebar width varies between 200 and 250 pixels, making it impossible to adapt the footer to this change without integrating it into the rustdoc main container directly. Another interesting change that was added to the sidebar was position: sticky which allowed me instead to move the footer completely at the bottom and to add margin-bottom on the sidebar. Here is a video of the change when you scroll to the end of the page:

Peek.2022-01-22.21-30.mp4

cc @syphar @jsha

@jsha

jsha commented Jan 22, 2022 via email

Copy link
Copy Markdown
Contributor

@jsha

jsha commented Jan 22, 2022 via email

Copy link
Copy Markdown
Contributor

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

I'm not sure about this. Needs to ask others what they think about it. A big problem is that the top navbar is already very "heavy" content-wise.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Also I'm not super attached to the rustdoc sidebar varying width. It seemed maybe nice to give back some more space on smaller screens and narrower docs, but if it causes other design headaches we can just ditch it.

The problem is that some crates already use this new style. And also, I personally like it (but it's just my opinion).

@syphar syphar added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jan 23, 2022
@syphar

syphar commented Jan 30, 2022

Copy link
Copy Markdown
Member

r? @Nemo157

@Nemo157

Nemo157 commented Jan 30, 2022

Copy link
Copy Markdown
Contributor

Empirically it appears that this style works on docs from 2021-12-14 just fine if .sidebar-menu also allows .sidebar-elems. It'd be good to reduce the variations if possibe.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Empirically it appears that this style works on docs from 2021-12-14 just fine

You mean 2021-12-05?

if .sidebar-menu also allows .sidebar-elems. It'd be good to reduce the variations if possibe.

You mean having this rule instead: .sidebar-elems, .sidebar-menu { ... }?

@Nemo157

Nemo157 commented Jan 30, 2022

Copy link
Copy Markdown
Contributor

You mean 2021-12-05?

Well, 2021-12-14 is one of the versions that would use the 2021-12-05 css and happened to be the one I found first while looking for a crate to test on.

You mean having this rule instead: .sidebar-elems, .sidebar-menu { ... }?

Yep. In fact, .sidebar-menu doesn't appear to exist at all in the rustdoc html?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

It existed in 1.58: https://doc.rust-lang.org/1.58.0/std/

@Nemo157

Nemo157 commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

Yep, but not in 2022-01-19+ that this css is targeting (or I think in 2021-12-05+ that the previous css was targeting).

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

I updated the date to 2021-12-14.

@syphar

syphar commented Feb 6, 2022

Copy link
Copy Markdown
Member

@GuillaumeGomez @Nemo157 how can we proceed here?

I see a test is failing, what else is missing?

@syphar syphar added S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Feb 6, 2022
@Nemo157

Nemo157 commented Feb 6, 2022

Copy link
Copy Markdown
Contributor

The reason I brought up this potentially working with all post-2021-12-05 docs is that we really need to minimize the number of css variations we have. The more we need to maintain the harder it becomes to do any changes that affect them (especially as we don't have a really easy way to test older versions outside deploying to production and checking the prebuilt docs).

My assumption is that if it is a change from 2022-01-19 that prompts this change, and 2021-12-14 works, then everything from 2021-12-05 -- 2022-01-19 will probably work too, and we can just change the 2021-12-05 css and not introduce a new variation.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Finally took the time to come back to this. @Nemo157 was right: it works just fine with 2021-12-05 crates as well.

@syphar

syphar commented Feb 13, 2022

Copy link
Copy Markdown
Member

@GuillaumeGomez thanks for checking!

so this is ready for another review? perhaps by @Nemo157 ?

@Nemo157 Nemo157 merged commit ee84c34 into rust-lang:master Feb 13, 2022
@github-actions github-actions Bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Feb 13, 2022
@GuillaumeGomez GuillaumeGomez deleted the fixes-sidebar branch February 13, 2022 12:05
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Footer is broken with new sidebar changes

4 participants